Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix for handling changes in File::Find 1.41 on win #2113

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

mikemagowan
Copy link

Summary

In a relatively recent update to File::Find v1.41 (which was included in the Perl 5.38 release) a change was made to how the windows directory separator (backslash) is handled and is now converted within File::Find to a forward slash to make it consistent. This was done in the below commit:

Perl/perl5@414f14d

This now causes several tests to fail in Mojo::File with Perl 5.38 on Windows.

Motivation

Mojolicious will not past tests on Windows with Perl 5.38.

References

#2105

@kraih kraih requested review from a team, marcusramberg, kraih, jberger and Grinnz September 20, 2023 16:07
Copy link
Member

@jhthorsen jhthorsen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can a test be written for this?

@mikemagowan
Copy link
Author

There are two consequences of this bug on the returned collection:

  1. The requested directory is included in the returned collection as the delete fails to match the key
  2. The sort order of the collection changes (backslashes vs slashes)

When I considered a test for the two things above it felt redundant as this is already tested in the tests that fail.

I'm open to suggestion though if you have a better idea for a test for this?

lib/Mojo/File.pm Outdated
@@ -63,17 +63,18 @@ sub list_tree {

# The File::Find documentation lies, this is needed for CIFS
local $File::Find::dont_use_nlink = 1 if $options->{dont_use_nlink};


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this empty line change?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure how that whitespace crept in - now removed

@kraih
Copy link
Member

kraih commented Sep 22, 2023

Squash your commits.

In a relatively recent update to File::Find (which was included in the Perl 5.38 release) a change was made to how the windows directory separator ('\') is handled and to convert this within File::Find to '/' to make it consistent:

Perl/perl5@414f14d

This then causes tests to fail in Mojo::File with Perl 5.38 on Windows.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants